perf(render): --photoreal in seconds, not minutes (rose-pro 184.7 s → 7.2 s warm) - #825
Conversation
… the BRep A photoreal render was paying full kernel evaluation on every invocation. rose-pro is 137 scene roots of bent sheet metal, and re-deriving all of them took most of the wall time of a --spp 64 --size 1200 render -- the same geometry, from the same unchanged source, every single run. The root-mesh cache (cad60a7) already solves exactly this for the raster and SVG paths, but root_cache() excluded --photoreal on the grounds that the path tracer needs analytic BRep surfaces. That is a choice, not a constraint: the tracer is perfectly happy intersecting triangles, and already did so for mesh-only parts. So invert the default. --photoreal now traces a 128-segment tessellation, which the cache can serve; --exact opts back into analytic ray-surface intersection, and therefore out of the cache. Two things fell out that I did not expect: * Tracing triangles is much *cheaper* than analytic BRep intersection here -- 4.4x less CPU (884s user -> 199s). So even a cold, empty-cache run comes out a third faster than before. The cache is the headline, but it is not the only win. * Peak RSS on a warm run drops 40% (867 MB -> 536 MB), because a cache hit never materialises a BRep at all. Contract decisions: * Mesh mode tessellates on a cache *miss* too, rather than using the BRep it happens to have in hand. A render must not change its pixels depending on whether an accelerator is populated, and the benchmark below confirms cold and warm output is byte-identical. * For the same reason PhotorealOptions::exact defaults to false for every caller, not just the CLI -- animate.rs included -- so all photoreal output agrees regardless of who asked for it. A caller with no cache installed pays a small quality cost for that consistency; that is the right trade. * MESH_SEGMENTS is fixed at 128 for all canvas sizes, rather than scaling like the raster path. The tracer resolves silhouettes far more sharply than the flat raster shader, so 64 shows at sizes the raster path gets away with; the extra triangles cost almost nothing (BVH traversal is logarithmic); and a size-independent count means one cache entry per root serves every --size. Also fans the per-solid BVH builds out over rayon, which is the one genuinely parallel stage of scene setup and matters most on a many-root assembly. Measured on an Apple M4 Max (16 core, macOS 26.5), /usr/bin/time -l, hardware/rose-pro/rose-pro.loon (137 roots) at --photoreal --spp 64 --size 1200, VCAD_CACHE_DIR pointed at a fresh directory for each cold run: | run | wall | user | peak RSS | |--------------------|----------|---------|----------| | before, cold | 186.02 s | 883.9 s | 887 MB | | before, warm | 184.69 s | 857.9 s | 867 MB | | after, cold | 125.72 s | 198.6 s | 944 MB | | after, warm | 5.96 s | 78.4 s | 536 MB | | after, --exact | 195.41 s | 876.9 s | 894 MB | And on examples/skin-demo.loon (from claude/loon-skin-shell, benchmarked in a throwaway merge worktree that has since been removed) at --photoreal --spp 32, default size -- only 3 roots, and trace-bound rather than eval-bound: | run | wall | user | peak RSS | |--------------------|----------|----------|----------| | before, cold | 138.63 s | 1411.8 s | 2037 MB | | before, warm | 189.76 s | 1389.9 s | 2152 MB | | after, cold | 50.39 s | 446.0 s | 2059 MB | | after, warm | 33.96 s | 432.2 s | 1664 MB | | after, --exact | 120.06 s | 1431.8 s | 2144 MB | That scene shows where the win comes from when the cache has little to do: 3 cached roots only buy cold->warm 50 s -> 34 s, but tracing the skin as triangles instead of analytically is a 3.2x CPU cut on its own. "before, warm" is not a typo: the old binary bypassed the cache for --photoreal, so a second run bought it nothing. The warm number people actually feel is 184.7 s -> 6.0 s, a 31x improvement; cold is 1.5x. Caveat on the numbers: this machine was running other people's builds during the measurement, so absolute times carry real contention noise -- skin-demo's two cacheless "before" runs, which should be identical, came out 138.6 s and 189.8 s, so treat single numbers as +/- 35%. Each before/after pair was run back to back at identical settings, and every ratio claimed here is far larger than that drift. Equivalence, all checked by byte comparison: * cold == warm output (the cache is a pure accelerator) * --exact == the pre-change binary's output, byte for byte, so the analytic path is provably untouched * default != --exact, as expected -- they are different geometry Mesh vs exact, eyeballed at 1200px: mean absolute difference 2.0/255 with 2.2% of pixels differing by more than 8. The difference is not one-sided. Mesh mode facets the rims of the D55 actuator cans, visibly; exact mode shows faint banding across large planar faces that crease-baked normals do not, and the framing shifts a hair because the camera fits bounds a tessellation inscribes. Mesh loses on curves, wins on creases. Both are documented on the flag. Gates: cargo test --workspace --exclude vcad-desktop --features vcad-kernel-text/no-builtin-font green (322 ok, 0 failed); clippy -p vcad-render -p vcad-kernel-raytrace -D warnings clean; the raster path byte-identical before and after, both with and without the cache; photoreal_animation.rs's "evaluated exactly once" assertion still holds unmodified (no evaluation was added or moved). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Photoreal changes to the path tracer had no quality gate: "it still looks right" was the whole acceptance test. That is fine for a rendering change you can eyeball and useless for a sampling change, where the whole point is to move noise around without moving the picture. Adds two pieces: - `cargo run --release -p vcad-render --example psnr -- ref.png cand.png [--min-psnr N]` — PSNR over all three 8-bit channels and mean grayscale SSIM over 8x8 windows at stride 4. Exits non-zero below the floor, so it drives a shell gate directly. Unit-tested against closed-form values (identical => inf/1.0; uniform 1-LSB offset => 20*log10(255) dB). - `scripts/photoreal-quality.sh` — renders each scene at a high reference spp and at the normal candidate spp, then scores them. References are multi-MB PNGs and stay OUT of the repo (default /tmp/vcad-photoreal-ref), regenerated on demand or with --regen. `EXTRA_ARGS` sweeps a flag under test against the same references. Two scenes, both flavours (raw film and default-denoised). The gate scores the *denoised* flavour, because that is the image a user sees; the raw film is reported for information only, since at 32spp it is honest Monte Carlo noise against a 1024spp reference and would fail any useful floor. Baseline, 32spp candidate vs 1024spp reference, --seed 7, Apple M4 Max: scene variant PSNR SSIM rose-pro raw 35.84 dB 0.86746 rose-pro denoised 40.73 dB 0.99409 plate raw 37.23 dB 0.89412 plate denoised 45.50 dB 0.99316 Acceptance rule for the sampling work that follows: denoised PSNR >= 35 dB against the 1024spp reference, and never worse than these numbers at equal spp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`render` fanned out one rayon task per scanline over six zipped `par_chunks_mut`. Tiles are a better unit of work for a path tracer — cost per pixel varies by an order of magnitude, and a full-width row averages that variation away, leaving every task the same size and work-stealing nothing to steal. A square tile keeps the cheap background and the expensive fillet interior in *different* tasks, and 256 neighbouring rays touch the same corner of the BVH. The pixel loop moves into `trace_pixel`, seeded from (px, py, seed) exactly as before, so the decomposition is invisible in the output. Tiles are traced into local buffers and blitted afterwards, which needs no aliasing tricks over the shared film. **Verified byte-identical.** All four harness renders (rose-pro and plate, raw and denoised, --spp 32 --seed 7) hash the same before and after, and PSNR/SSIM are unchanged: scene variant PSNR SSIM md5 vs scanline rose-pro raw 35.84 dB 0.86746 identical rose-pro denoised 40.73 dB 0.99409 identical plate raw 37.23 dB 0.89412 identical plate denoised 45.50 dB 0.99316 identical **Wall time: neutral.** Measured back-to-back, alternating binaries, Apple M4 Max, other builds idle. Run-to-run variance (~1.5 s on the rose-pro pairs) is larger than the difference, so this is not a speed win: scene / config base tiled rose-pro 800px 128spp 9.90 / 11.27 / 10.41 10.23 / 11.20 / 9.95 rose-pro 1600px 64spp 20.18 / 19.69 / 18.01 20.11 / 19.04 / 17.53 plate 256px 512spp 2.05 / 2.03 1.93 / 2.21 That is the honest result and it is unsurprising: `par_chunks_mut` already splits adaptively, and at 800-1600 rows there are far more scanlines than cores, so the existing decomposition was not the bottleneck. Tiling is kept because it is the substrate the adaptive-sampling work needs — a stopping decision has to be made over a coherent 2D neighbourhood, not a stripe — and because it costs nothing. Adds `ragged_frame_leaves_no_untraced_seam`: a 37x23 frame straddles the tile grid on both axes, and an off-by-one in the blit would leave a black stripe the determinism test would happily reproduce. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ion) Pixel jitter and lens position were four fresh uniforms per sample. Four uniforms clump: at 32spp a purely random jitter routinely leaves part of the pixel footprint uncovered and doubles up elsewhere, which is aliasing paid for at full sample cost. They now come from a rotated 4D Hammersley set — `(s + 0.5)/spp` paired with the base-2, base-3 and base-5 radical inverses — offset per pixel by a Cranley-Patterson rotation drawn from the existing PCG. Every pixel shares one low-discrepancy set; the per-pixel rotation keeps its stratification while decorrelating neighbours, so the residual reads as noise instead of a pattern locked to the pixel grid. Determinism is untouched: the rotations come from the same per-pixel seed, in a fixed order. The RNG draw order changes (four rotations up front, none per sample), so historical per-pixel values move. Nothing pinned them; `deterministic_ across_runs` and the denoise RMSE tests assert properties, not values, and all still pass. **Quality: a wash, and worth saying so plainly.** Five seeds, plate at 16spp un-denoised, against the same converged 1024spp reference: seed random jitter Hammersley 1 34.14 dB / 0.81477 34.16 dB / 0.81499 2 34.13 dB / 0.81460 34.21 dB / 0.81552 3 34.07 dB / 0.81379 34.14 dB / 0.81473 4 34.12 dB / 0.81431 34.15 dB / 0.81459 5 34.18 dB / 0.81513 34.14 dB / 0.81467 mean 34.13 dB 34.16 dB (+0.03 dB) Wins 4 of 5 seeds by about the width of the seed-to-seed spread. The single-seed harness numbers at --seed 7 land the other way (rose-pro raw 35.84 -> 35.72 dB) which is what motivated the five-seed run: at this effect size one seed says nothing. The reason the win is so small is worth recording for whoever picks this up: on these scenes essentially none of the variance lives in the camera dimensions. It is in the light-choice and BSDF dimensions inside `radiance`, which are still plain uniforms. Stratifying those means threading a sample index through the recursive path and is a much larger, riskier change against MIS — left for later. Kept anyway: it is free (wall time identical, 15.17/14.63 s before vs 15.41/14.29 s after on rose-pro 800px 128spp, alternating runs), it is not a regression by any measure, and it is the correct construction for the antialiasing it does govern. Also adds `BIN=` to scripts/photoreal-quality.sh so a candidate can be A/B'd against a baseline binary over one shared set of references — without it the script's own `cargo build` clobbers the binary under test, which silently produced four identical comparison rows before it was caught. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tive) Every pixel got exactly `--spp` samples, whether it needed them or not. Most do not: a flat lit face converges in a handful of samples and then spends another hundred confirming it, while the caustic-ish corner next to it is still noisy at the end. The Film already computed each pixel's variance of the mean — the estimator's own error bar — and threw it away except as a denoiser guide. `trace_pixel` now samples in batches of 16 and, between batches, checks the 95% confidence half-width of pixel luminance against a relative tolerance with an absolute floor (`ci <= 0.10 * (mean + 0.02)`), stopping early when it passes. `--spp` becomes a ceiling. Every pixel gets at least 32 samples regardless: a pixel that draws several near-equal samples early reports a tiny variance and would quit while genuinely unconverged, which is the classic adaptive-sampling blotch. The decision is made per pixel from that pixel's own running sums — never from a tile or neighbourhood — so the film stays deterministic and independent of the tiling. Verified: two runs of rose-pro at --seed 7 --spp 128 hash identically. The camera point set switches Hammersley -> Halton (base 2/3/5/7). Hammersley's first dimension is `s / N`, which needs the final sample count up front; adaptive sampling does not know it, and a set that changes shape when the loop stops early is worse than a slightly weaker set that is correct at every prefix. **Tolerance sweep**, rose-pro 800px --spp 128 --seed 7, denoised PSNR vs the 1024spp fixed-count reference: ADAPTIVE_TOL PSNR wall vs fixed off (fixed) 45.04 dB 9.6 s — 0.05 44.77 dB 8.0 s -17% 0.10 44.54 dB 6.4 s -34% 0.20 43.61 dB 4.7 s -51% 0.10 chosen: 0.20 gives up half a dB for a saving that a plain spp cut would nearly match, while 0.05 barely earns its complexity. **Equal-quality comparison** — the number that actually matters. Fixed sampling was dialled down until it matched adaptive's quality: config denoised PSNR wall (3 runs) fixed --spp 112 44.50 dB 10.84 / 10.34 / 11.46 s adaptive --spp 128 44.54 dB 6.34 / 6.69 / 6.30 s Slightly better picture in ~40% less wall time. (~1.2 s of each figure is geometry evaluation, not tracing, so the tracing-only saving is larger.) **Full harness at --spp 128**, both scenes: scene variant fixed adaptive rose-pro raw 41.02 / 0.95291 38.62 / 0.90383 rose-pro denoised 45.04 / 0.99670 44.54 / 0.99589 plate raw 42.44 / 0.96248 40.02 / 0.93044 plate denoised 49.08 / 0.99593 48.14 / 0.99494 All well above the 35 dB gate. Note how much smaller the denoised gap is than the raw one: adaptive sampling leaves its residual error exactly where the variance estimate is large, which is exactly where the variance-guided denoiser is most willing to filter. The two compose well, which is the main argument for defaulting this on. **On by default**, with `--no-adaptive` to opt out. The quality cost is 0.3-0.9 dB at the default 128spp against a 35 dB floor, the saving is ~40% of wall time at matched quality, and the failure mode (a pixel stopping early) is bounded by the floor. `--no-adaptive` exists because a reference render needs a uniform sample count to mean anything — and scripts/photoreal-quality.sh now passes it when generating references, for exactly that reason. Timing caveat: the numbers above were taken back-to-back, alternating binaries, on an Apple M4 Max. An earlier pass with concurrent builds running showed the same fixed run at 16-18 s rather than 9.6 s; absolute figures move a lot under load, ratios within a back-to-back pair much less. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`trim::point_in_face` rebuilt the face's entire UV trim boundary on every
single ray-face hit test. For one point-in-polygon query it would:
- walk the outer loop and inverse-project every vertex onto the surface
(Newton iteration for B-spline and bilinear faces),
- allocate a Vec for that, another for `repair_pole_vertices`, another for
each inner loop,
- re-derive the degenerate-cap polygon from the adjacent surface,
- and recompute the unbounded-v clamp.
None of that depends on the query point. A frame asks the question millions
of times and threw the answer away every time.
The boundary now lives in a `FaceTrim` — outer polygon, untrimmed flag,
v-range, hole polygons — built once per face when the BRep BVH is
constructed and stored alongside `faces`. `point_in_face` is kept as
`FaceTrim::build(..).contains(..)` for the many callers outside the tracer
(vcad-kernel-booleans has its own copy and is untouched).
Pure caching: no arithmetic changed, only when it happens. Verified
byte-identical — parametric-plate `--photoreal --exact --spp 64 --size 400
--seed 7` hashes the same before and after.
**Measured**, Apple M4 Max, alternating binaries back-to-back, three pairs:
parametric-plate --exact --spp 64 --size 400
before after
real 8.58 / 7.00 / 5.30 s 1.14 / 0.86 / 0.48 s
user 28.54 / 29.43 / 29.34 s 4.85 / 4.84 / 4.92 s
Real time on this machine swings by a factor of 1.6 run to run; CPU time
does not, and it is the honest figure: **6.1x less work**.
rose-pro --exact --spp 4 --size 200
before after
real 156.10 s 159.18 s
user 149.98 s 147.34 s
**No change on rose-pro, and the reason matters.** Nearly all of that
150 s is BRep evaluation and BVH construction — a `--exact` render cannot
cache a `vcad_kernel::Solid`, so it re-evaluates the kernel every run — and
of the geometry that remains, most of rose-pro's 137 solids fall back to
the mesh boolean ("rejecting the B-rep result and re-cutting with the mesh
boolean") and therefore trace as `BvhGeom::Mesh`, which never reaches this
code. The plate is the scene that actually exercises analytic BRep tracing,
and it is where the 6x shows up.
An earlier draft of this message quoted 427 s -> 358 s for rose-pro. Those
runs were taken while the disk was at 100% and the process was I/O-stalled
(real 427 s against ~150 s of CPU); they are discarded, not corrected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`intersect_surface` heap-allocated a `Vec<SurfaceHit>` per ray-surface test
— millions per frame — to hold at most four elements. Every analytic
intersector did the same internally.
They now return `SurfaceHits = SmallVec<[SurfaceHit; 4]>`. Four is exact,
not a guess: a torus is the worst analytic case at four roots. A B-spline
can exceed it and spills to the heap, which is fine — that path is already
dominated by Newton iteration and subdivision.
Byte-identical output (parametric-plate `--exact --spp 64 --size 400
--seed 7`, same hash as before the trim-cache change too).
**Measured on top of the trim cache**, three pairs, CPU time (real time on
this machine is too noisy at this scale to say anything):
parametric-plate --exact --spp 64 --size 400
trim cache only 4.85 / 4.84 / 4.92 s user
+ SmallVec 4.71 / 4.61 / 4.71 s user
About 3%, consistent in sign across all three pairs but small enough that
it should be read as "a little less work" rather than a headline. With the
trim rebuild gone the remaining per-test cost is the intersection
arithmetic itself.
Kept because it is strictly less work in the hottest loop in the crate, it
is contained (one type alias, seven call sites), and the allocation would
matter more on a part with many more analytic faces than these scenes have.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two paths into GpuScene used to swallow unsupported geometry and hand
back a scene that renders as empty space, with no diagnostic anywhere:
1. `Bvh::flatten()` returned `(vec![], vec![])` for a mesh-backed BVH.
`GpuScene::from_brep` then built a scene with one zeroed BVH node and
zero faces, uploaded it, and produced a blank frame. There is no
triangle BLAS in the WGSL tracer yet, so this is a real missing
capability, not an empty scene.
2. `GpuSurface::from_surface` packs bilinear as type 5 and B-spline as
type 6. The WGSL `intersect_surface` switch (shaders/raytrace.wgsl)
has cases for 0-4 only; 5 and 6 fall into `default`, which returns a
miss. Any face on such a surface silently vanished from the image
while the rest of the solid rendered normally -- the worst failure
mode, since the render looks plausible.
Both now fail loudly and name the offending geometry.
API changes (kept as small as the callers allow):
- `Bvh::flatten()` -> `Result<(Vec<FlatBvhNode>, Vec<FaceId>),
FlattenUnsupported>`. The new error carries the triangle count. Only
two callers exist: `GpuScene::from_brep` and the bvh unit tests.
- `GpuSceneError` gains `UnsupportedSurface { index, surface_type, name }`
and `UnsupportedMeshGeometry(FlattenUnsupported)`, plus a `From` impl
so `from_brep` can `?` the flatten result. `UnsupportedSurface` names
the surface index within `brep.geometry.surfaces` and the type name,
so a caller can report *which* face it cannot draw.
- `GpuSurface::is_gpu_traceable()` and `GpuSurface::type_name()` are the
single place that encodes "which type codes the shader switch handles".
If a new `intersect_*` case lands in the WGSL, bump SURFACE_TYPE_TORUS
and the validation follows.
The wasm caller needs no change: `vcad-kernel-wasm`'s `uploadSolid` and
`uploadSolidWithMaterial` already `map_err` `GpuSceneError` into a
`JsError` carrying the Display string, so the browser viewport now gets
"surface 3 is a BSpline (type 6), which the GPU tracer cannot intersect"
in place of a mystery blank canvas, and can fall back to the raster path.
Tests: mesh flatten errors (both populated and empty mesh), BRep flatten
still round-trips face IDs, the 0-4 / 5-6 traceable split, a cube still
builds a scene, and a cube with one plane swapped for a BilinearSurface
is rejected with the surface index and name in the error.
Gates: cargo test -p vcad-kernel-raytrace --features gpu --lib (115 ok),
clippy --all-targets -D warnings clean, and
cargo check -p vcad-kernel-wasm --target wasm32-unknown-unknown green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ack HDR once
`render_with_render_state` is shaped for the browser viewport: one sample
per call, driven by a refinement scheduler that resets on every camera
gesture. Each call therefore recreates every scene buffer (surfaces,
faces, BVH, trim verts, inner-loop descs, materials, lights, env
textures), rebuilds the bind group, and reads back the tonemapped
Rgba8Unorm texture. At 1 spp/frame that is the right shape. At 512 spp
it is 512 scene uploads and 512 GPU->CPU round trips, and it hands back
8-bit sRGB -- already tonemapped, so the CPU side cannot apply its own
exposure without undoing an ACES curve first.
`render_offline` is the offline shape of the same kernel:
- scene buffers uploaded ONCE, bind group built ONCE;
- the per-sample loop rewrites only the 128-byte RenderState uniform
(frame index + Halton jitter) and dispatches the existing `main`
kernel;
- ONE readback at the end, of the f32 accumulation buffer (binding 8,
vec4<f32>) rather than the texture, so the caller gets linear HDR
radiance and applies exposure/ACES/sRGB itself.
Measurements (M-series, Metal, release, 512x512 @ 128 spp, sphere scene,
median of three runs):
render_offline 46.6 ms 364 us / spp
render_with_render_state x N 184.3 ms 1.440 ms / spp
speedup 3.9x
Note the scene here is a single-surface sphere, so almost none of that
3.9x is the scene upload -- it is the per-sample readback and bind-group
churn. A scene with thousands of faces widens the gap further.
Design decisions:
- One submit per sample, not one encoder for the whole loop.
`queue.write_buffer` is staged and applied at the next submit, so
several dispatches inside one encoder would all read the same frame
index and collapse the running mean into a single sample.
- The viewport's refine and denoise passes are skipped. Both trade bias
for perceived quality at low sample counts, which is exactly the wrong
trade when the point is to converge. (The shader's denoise only ever
touched the output texture, never `accum_buffer`, so the HDR readback
was already clean -- but the refine pass does write accum, hence
`refine_sample_count = 0`.)
- `max_depth` is held constant instead of using `depth_for_frame`. The
escalation exists to make the viewport's *first* frame land fast; in a
running average it just biases the estimate toward the shallow early
samples.
- Deterministic seed: `GpuRenderState` gains a `seed: u32` in what was
the first of three padding words, so the uniform is still 128 bytes and
no layout changed. WGSL's `rand_uniform` folds `seed * 2654435761` into
its PCG hash. Every existing constructor sets `seed: 0`, and `+ 0`
reproduces the old hash bit for bit -- the browser path's noise is
unchanged.
- Native only (`cfg(all(feature = "gpu", not(target_arch = "wasm32")))`).
It blocks on `device.poll(Maintain::Wait)`, which deadlocks the
browser's single-threaded event loop; that is precisely why the
viewport entry points are async.
`OfflineResult::to_film` repackages the HDR buffer as a `pathtrace::Film`
so `Film::to_srgb8` -- the CPU renderer's own output transform -- can be
reused rather than reimplemented. Its guide buffers are zeroed, so the
doc comment warns it must not be fed to `pathtrace::denoise`.
Tests (tests/gpu_offline.rs, `#[ignore]`-tagged and adapter-skipping like
gpu_smoke.rs):
- offline_hdr_matches_cpu_mean_luminance: 64 spp, HDR is finite and
non-negative, non-black, non-constant, and its mean luminance lands
within a factor of two of `pathtrace::render` over an equivalent CPU
scene (same solid, same material, same studio rig off the same BVH
root bounds, same gradient env, no ground). The subject is framed to
overfill the viewport on purpose -- the GPU's `sky_color` backdrop is
a themed UI choice and differs from the CPU's, while `env_radiance`
(the lighting) is shared, so a frame with visible background would
compare two different things. The test asserts >=98% coverage so that
assumption fails loudly if the framing ever drifts.
- offline_render_is_deterministic_for_a_fixed_seed: same seed twice ->
identical buffers; different seed -> different buffer, which is what
proves the new seed field actually reaches `rand_uniform`.
- more_samples_reduce_noise: 64 spp is measurably smoother than 1 spp,
the cheapest end-to-end proof the loop averages rather than
overwrites.
- bench_offline_vs_viewport_loop: the measurement above.
Not in this slice, deliberately: vcad-render is not wired to the GPU, and
there is still no triangle BLAS.
Gates: cargo test -p vcad-kernel-raytrace --features gpu green (115 unit
+ all integration; the four gpu_offline tests and gpu_smoke pass under
--ignored on this adapter), bsdf_parity unchanged, clippy --all-targets
-D warnings clean, cargo check -p vcad-kernel-wasm
--target wasm32-unknown-unknown green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`--photoreal` traces cached triangle meshes by default, so the GPU path
tracer was unavailable for exactly the geometry the renderer feeds it:
`Bvh::flatten` errored on a mesh-backed BVH and `GpuScene` had no way to
build one. This adds the triangle to the WGSL tracer and removes that
fail-closed seam (`GpuSceneError::UnsupportedMeshGeometry` is gone).
Packing: a triangle rides inside an existing `GpuSurface`'s `params`
block under a new type code 7, rather than in storage buffers of its
own. The bindings are already at the browser cap of ten
(`maxStorageBuffersPerShaderStage` -- it is why the env map went into
textures), and 3 positions + 3 normals + a flag is 19 of the 32 idle
floats. So the mesh path costs zero new bindings and works in the
browser without forking the shader.
The honest cost is de-indexing: shared vertices are stored once per
incident triangle, 144 B/tri flat, so 500k tris is ~72 MB of surface
buffer (measured: 76k tris -> 10.5 MB). On unified memory that is a
fair trade; a native-only indexed path with raised limits would be
roughly 4x leaner and is the obvious follow-up if it ever bites.
Normals: `params[18]` flags whether the packed shading normals are
real. The shader cannot tell an absent normal from a zero one, and
would normalize the latter into NaN across the whole surface -- so the
flag is what selects the geometric-normal fallback. Otherwise the
corner normals are blended barycentrically, mirroring `MeshGeom::test`
on the CPU including its second fallback (a blend that cancels across a
degenerate crease). `compute_tangent` leaves type 7 in the default arm:
a triangle has no dP/du, so the shading frame falls back to an
arbitrary basis, which is what an isotropic BSDF wants anyway.
`Bvh::flatten` now returns `(nodes, FlatPrims)` and is infallible;
`FlatPrims` is `Faces(Vec<FaceId>)` or `Triangles(Vec<FlatTriangle>)`,
so the consumer learns from the value which it got. The recursive
flattener is generic over the primitive, so both arms share one copy of
the leaf index bookkeeping.
`GpuScene::merge` now re-derives the studio rig from the merged root.
Keeping self's rig lit a merged scene as if only half of it existed --
which is precisely the mixed BRep+mesh case this slice enables.
Measured (M-series, 512x512 @ 128 spp, 76,288-triangle icosphere):
GPU 349 ms
CPU 16.5 s -- 47x
Parity, mesh icosphere at 64 spp, GPU vs the CPU tracer over the SAME
`Bvh::build_mesh` tree (so tessellation error, which both share, is out
of the picture): mean luminance ratio 1.000, PSNR 29.8 dB. The test
gates at 25 dB -- loose because the two integrators share a BSDF and a
light rig but not their sampling (different RNG, different MIS
bookkeeping, f32 against f64), and two independently noisy estimates of
the same signal differ by about the sum of their variances. A real
defect lands far below it: dropped shading normals ~20 dB, a broken
intersector in the single digits. Mixed BRep+mesh scene: 23.8 dB over
subject pixels (masked, since the two renderers draw different
backdrops by design).
Not wired into vcad-render yet.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Path-traces the photoreal scene on the wgpu compute pipeline instead of on rayon. Same scene, not a second renderer: geometry from `build_objects`, framing from `frame_view`, lights/env/floor from `dress_scene`, and the film back through `Film::to_srgb8`, so exposure, ACES and sRGB encoding are the CPU path's byte for byte. Only the integrator changes. Behind `vcad-render/photoreal-gpu` (off by default -- wgpu and its backends are a heavy dependency for a CLI most people run on the CPU). A build without the feature still *has* `--gpu` and answers "not compiled in", so the diagnostic points at the build rather than at the flag. An explicit `--gpu` with no adapter is a hard error, never a quiet CPU fallback: a user who asked for the GPU and silently got the CPU would misread every timing afterwards. What `--gpu` honours, and what it refuses ---------------------------------------- Honoured, matching the CPU path: --spp --max-depth --exposure --fov --seed --size --fill --auto-aspect --view/--azimuth/--elevation (including the mirrored isometric basis), --env and --env-rotation (gradient and HDRI), per-part materials, --backdrop studio|none. Refused with a message naming the flag, rather than rendering something else: --exact GPU BRep path caps at ~1k analytic surfaces --aperture the WGSL camera is a pinhole --ortho the WGSL camera is projective only --backdrop shadow-catcher no shader counterpart --animate poses are baked into uploaded vertices Ignored, and said so: --no-adaptive (no GPU adaptive sampler, so --spp is an exact count not a ceiling) and the denoiser. The denoiser is guided by the per-pixel normal/depth/albedo the CPU integrator records; `render_offline` reads back radiance only, so `to_film` leaves those zeroed -- and `denoise` passes `depth == 0` pixels through untouched. Run on a GPU film it would not produce a worse image, it would silently produce *no filtering at all*. So --gpu turns it off and prints why. Writing the guide buffers from the shader is the obvious follow-up. The five gaps the BLAS slice left open -------------------------------------- Per-object materials. `from_mesh_bvh` hard-coded material 0. Added `from_mesh_bvh_placed(bvh, material, transform)`; `merge` already rebases `material_idx`, so each solid keeps its own Pbr. Object transforms. The shader walks one flat node array with no instancing layer, so a per-object matrix has nowhere to live. Transforms are baked into the packed vertices at scene-build time: positions through `apply_point`, shading normals through `apply_normal` (the inverse-transpose -- `apply_vec` would shade a non-uniformly scaled part wrong), BVH node AABBs re-fitted around their transformed corners (conservative under rotation, never lossy). Identity is detected and skipped so the common case packs bit-identically. Baking is why --animate stays on the CPU. Camera parity. `Camera::from_basis` can carry a MIRRORED screen basis -- `View::Isometric` and every named CAD view do -- and the shader's `right = forward x up` reconstruction cannot represent one, so a GPU isometric would render flipped. `GpuCamera` gained `right` and `basis_mode`; `camera_basis()` in WGSL uses the supplied axes verbatim in explicit mode and is unchanged (mode 0) for the viewport. `the_isometric_view_is_not_mirrored` pins it with a column-profile comparison, which catches a flip that a whole-image metric would not. Caps. `merge` never validated anything and `MAX_BVH_NODES` was 8192 -- rose-pro needs 606,845. Raised to 4M (nothing in the shader is sized by it; the real ceiling is the device's `max_storage_buffer_binding_size`, now checked explicitly). Added `GpuScene::validate`, `bvh_depth`, and `merge_all`, which folds N parts pairwise instead of linearly: rose-pro's 137 parts cost 7 levels rather than 136. The traversal stack went 32 -> 64 and, critically, is now *checked*: overflow in `trace_bvh` silently drops the push, so an over-deep scene used to lose geometry rather than fail. rose-pro merges to depth 46 -- comfortably over the old 32. Backdrop and environment parity. The GPU always drew `sky_color`, a themed viewport backdrop unrelated to the sky it lights with. Added `RenderState.background_mode`: 0 = sky (viewport, unchanged), 1 = `env_radiance` (what --photoreal shows), 2 = black (the CPU's `show_background = false`, which with the coverage alpha gives a transparent PNG). It has to be shader-side -- a silhouette pixel has already averaged background and surface samples together. The studio floor is uploaded as a real quad at `framing.floor_z` with the CPU's `Ground` material, so it shadows and bounces through the same BSDF; the CPU's plane is infinite and this one is 50 scene-radii across (600 showed f32 self-shadow banding across the floor -- measured, not guessed). Two shader bugs found on the way -------------------------------- * The primary-ray `intersect_ground` call was ungated, so `ground_enabled = 0` still drew the implicit z=0 floor -- invisible in the viewport, which always enables it, and a second floor slicing through rose-pro's legs offline. * `accumulated.a = 1.0` unconditionally clobbered the integrator's coverage estimate with the refine pass's sample-count marker. Now written only when refinement is on, which is what nothing else reads it. `--backdrop none` was coming back fully opaque for exactly this reason. Benchmark --------- rose-pro.loon, warm geometry cache, M-series unified memory, `/usr/bin/time`. Wall clock, best of three where noted; CPU numbers vary +-3%, GPU +-5% (the GPU's fixed cost is scene packing, which competes with the page cache). 1200px, 64 spp wall RSS CPU, default (adaptive + denoise) 7.19 s 621 MB CPU, --no-adaptive --no-denoise 9.88 s 568 MB GPU (equal effective spp) 4.23 s 1804 MB 1.7x / 2.3x marginal cost per sample, 1200px CPU 149 ms GPU ~50 ms 256 spp, fixed count CPU 38.6 s GPU 15.2 s 2.5x RSS is the honest cost: one `GpuSurface` per triangle is 144 bytes, so rose-pro's 867,838 triangles are ~125 MB of surface buffer before anything else. An indexed native path would be roughly 4x leaner. Quality, 800px, PSNR against the existing 1024-spp `--no-adaptive --no-denoise` CPU reference in /tmp/vcad-photoreal-ref: render wall PSNR SSIM CPU 64 spp, default 3.20 s 39.80 dB 0.988 CPU 64 spp, raw 4.19 s 38.40 dB 0.918 GPU 64 spp 1.94 s 29.11 dB 0.887 GPU 256 spp 7.45 s 29.81 dB 0.946 GPU 1024 spp 24.02 s 30.01 dB 0.963 The GPU converges -- to a slightly different picture. ~30 dB is a floor it does not cross with more samples: f32 against f64, plus the finite floor quad. That is stated in --help rather than buried: --gpu is for fast looks and sweeps, the CPU path is for the final hero. On the simple two-part test scene the two agree to 37 dB, so the gap scales with scene complexity. Tests ----- `crates/vcad-render/tests/photoreal_gpu.rs` renders the same document both ways and gates on PSNR (floor 24 dB against the ~37 dB measured, leaving room for independent Monte Carlo noise at test sample counts), plus the mirroring check and a `--backdrop none` transparency check. Skipped, not failed, without an adapter -- but only on the specific "no adapter" message, so a broken shader cannot skip its way to green. CPU photoreal output is byte-identical to the pre-change binary (verified by sha256 of a fixed-seed render against a stashed build); no CPU-path file is touched by this commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
| // dispatches inside a single encoder — they would all read the same | ||
| // uniform and collapse the running mean. | ||
| let (groups_x, groups_y) = (width.div_ceil(8), height.div_ceil(8)); | ||
| for frame_index in 1..=spp { |
There was a problem hiding this comment.
Minor · Structural quality — One GPU submit per sample means spp command-buffer allocations; batching a few dispatches per encoder would reduce driver overhead at high spp
The loop at line 1291 creates one encoder and one submit per sample, which is correct but leaves per-submit driver overhead on the table at high spp counts. Batching, say, 16 dispatches per encoder (with a write_buffer before each) would reduce that overhead without changing the accumulation semantics; this is a follow-up optimisation, not a correctness issue.
There was a problem hiding this comment.
Declining this one, with reasoning: batching dispatches per encoder is what the comment above the loop rules out. write_buffer is staged and applied at the next submit, so several dispatches sharing an encoder all read the same frame index and collapse the running mean — a write_buffer before each dispatch doesn't separate them, since none of them land until the submit.
There is a correct version of this — a pre-filled uniform array indexed by dynamic offset, one write and N dispatches — and it's worth doing, but it's a real change to the RenderState upload rather than a loop rearrangement, so I've left it as a follow-up. Worth noting the overhead is already modest at the sizes that matter: rose-pro at 1200 px runs ~50 ms/spp, against which a per-submit cost measured at roughly 0.4 ms/spp on a 512² scene is a few percent.
🤖 Addressed by Claude Code
Review follow-ups from #825. Both are comment-and-placement changes; rose-pro at 16 spp / 400 px / seed 7 hashes identically before and after on the CPU and GPU paths. The adaptive sampler's `.max(0.0)` on the sample variance reads as defensive and is not: with a Bessel-corrected estimator, `lsum2 / n` and `mean * mean` cancel to within f32 rounding once a pixel's samples agree, and the difference can land just below zero. That would put a NaN through the sqrt, and a NaN compares false against the tolerance, so the pixel would never converge and never stop. Say so, so nobody tidies the clamp away. `build_scene` derived the studio rig before merging the floor quad and assigned it after, which reads as an ordering dance between the two statements. It isn't one — the rig comes from `scene.lights`, not from the merge, so only the assignment's position matters: every `GpuScene::merge` re-derives the rig from the combined bounds, and the floor is hundreds of scene radii across. Deriving it at the point of assignment leaves one statement that has to stay after the last merge instead of two that have to stay in order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The stable and nightly jobs' format check was failing on this branch. Every hunk is rustfmt's own reflowing of code these commits introduced -- import ordering in gpu/mod.rs, array and method-chain wrapping, an assert! that now fits differently -- and no file outside the branch's own diff is touched. Renders hash identically before and after on both the CPU and GPU paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CI's clippy (1.98) has `chunks_exact_to_as_chunks`, which my local 1.97 does not, so this branch went red on a lint I could not reproduce until I installed the matching toolchain. Two sites were reported; running the lint with --all-targets and the gpu feature -- which CI's clippy invocation does not do -- found eight more in the GPU tests and in photoreal_gpu's own tests. The rewrite is a straight improvement rather than lint appeasement: as_chunks::<N>() yields &[T; N], so p[0..3] and v[2] are checked once by the type rather than on every access, and a stride typo becomes a compile error instead of a panic. rose-pro at 16 spp / 400 px / seed 7 hashes identically before and after on both the CPU and GPU paths. Workspace clippy and tests are clean on 1.98.0, the toolchain CI actually runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Choji review — Looks good · reviewed The delta is clean: No findings · review page Choji updates this comment as you push · Mention @chojiai in a comment to discuss, re-review, or request a fix |
vcad-render --photorealtook 30–60+ minutes on a 1200–1600 px, 128–256 spp render of a ~150-root document, and re-evaluated every root's BRep on each run because the path tracer consumed analytic surfaces and so bypassed the content-addressed root-mesh cache from cad60a7.rose-pro at 64 spp / 1200 px, warm cache: 184.7 s → 7.2 s on CPU (~26×), 4.2 s with
--gpu(~44×).What changed
Photoreal traces a cached tessellation. A path tracer only needs ray-intersectable surfaces, and mesh-source roots (
import-mesh,[skin]) never had analytic ones.--photorealnow runs inside the samewith_root_cachescope as the raster path, at a fixed 128 segments so one cache entry serves every--size.--exactopts back into analytic BRep intersection and is byte-identical to the pre-change binary.Two surprises worth stating: tracing triangles is 4.4× cheaper CPU than analytic BRep intersection, so even a cold run got 1.5× faster and
--exactis now the slowest option; and warm peak RSS drops ~40%, because a cache hit never materializes a BRep. A cache miss also tessellates rather than using the BRep in hand — cold and warm output are byte-identical, because a render must not change pixels based on accelerator state.Sampling. Rendering moved from per-scanline to 16×16 tiles (byte-identical; the substrate for what follows), camera dimensions are stratified with rotated Hammersley, and per-pixel adaptive sampling is on by default (
--no-adaptive): batches of 16 against a 95% CI, hard floor of 32 spp, seed-deterministic and tiling-independent. ~40% wall-time cut at matched quality (adaptive 128 spp = 44.54 dB in 6.4 s vs fixed 112 spp = 44.50 dB in ~11 s). Tiles and stratification measured as a wash on their own and are reported that way — the real variance lives in the light/BSDF dimensions insideradiance, which is left as follow-up work.--exacthot path.trim::point_in_facerecomputed each face's trim polygon on every ray/face hit test — reprojecting every loop vertex, Newton-solving for B-spline and bilinear. Those polygons are now built once per face at BVH build time: 6.1× less CPU on trim-heavy parts, byte-identical output. Surface hits also return aSmallVecinstead of a freshVec(~3%, marginal, reported as such).GPU. The wgpu tracer under
src/gpu/was real but browser-only: no triangle support, and it recreated every scene buffer and read back the tonemapped texture per sample. Now it hasrender_offline(upload once, accumulate N spp, read back HDR once — 3.9× per-spp), a triangle BLAS in WGSL packed into the existing surface array so the browser's 10-storage-buffer cap holds, andvcad-render --photoreal --gpubehind thephotoreal-gpufeature.Quality harness
Stochastic output can't be byte-compared, so
examples/psnr.rs(PSNR + grayscale SSIM) andscripts/photoreal-quality.shcheck candidates against 1024-spp references; references are regenerated into/tmp, not committed. Every sampling change here clears the 35 dB gate: rose-pro 64 spp scores 39.80 dB / 0.988 SSIM against its reference.On
--gpu, honestlyIt is ~3× per-sample over the CPU integrator and 47× over the CPU on a mesh-only microbenchmark — but it plateaus at ~30 dB against the CPU reference and does not cross it (f32 vs f64, plus a finite floor quad; the residual is not fully attributed). It also can't denoise (no guide buffers) or adapt, and the flat 144 B/triangle packing costs 1.8 GB RSS on rose-pro.
So the outcome is counterintuitive: the CPU default is both the quality path and nearly as fast, because the mesh cache and adaptive sampling ate most of the win the GPU was meant to deliver.
--gpuearns its place where raw spp throughput matters (2.5× at 256 spp fixed), and it refuses rather than guessing —--exact,--aperture,--ortho,--animate, and--backdrop shadow-catcherare hard errors, a missing adapter is a hard error, and the divergence is stated in--help. No silent fallbacks: one would poison every timing taken afterwards.Bugs found along the way
Bvh::flattenreturned empty vecs for meshes and the WGSL switch silently missed those surface types. Both now fail closed by name.GpuScene::mergekept the first scene's light rig, lighting a merged scene as if only half of it existed.ground_enabled, putting a second floor through rose-pro's legs; andaccumulated.a = 1.0clobbered coverage, so--backdrop nonecame back opaque.Numbers
rose-pro,
--photoreal --spp 64 --size 1200, M4 Max,/usr/bin/time -l:--gpu--exactexamples/skin-demo.loon(mesh-heavy, benchmarked against a throwaway merge withclaude/loon-skin-shell),--photoreal --spp 32: ~190 s → 34.0 s warm.Timing caveat stated in the commits: the machine ran other builds concurrently, and two runs that should have been identical differed by 35%. Every ratio claimed here is far larger than that drift.
Gates
cargo test --workspace --exclude vcad-desktopgreen;cargo test -p vcad-kernel-raytrace --features gpugreen including--ignoredon a real adapter; clippy--all-targets -D warningsclean on touched crates in both feature configurations; wasm32 check clean. The drafting and raster paths are byte-identical before and after, and CPU photoreal output is sha256-verified unchanged by the GPU work.Follow-ups
Guide buffers on the GPU to re-enable denoising (the likeliest route to closing the quality gap); indexed triangle upload (~4× less RAM); stratifying the light and BSDF dimensions, where the variance actually is; attributing the ~30 dB CPU/GPU floor; shadow-catcher and DOF have no shader counterpart yet.
🤖 Generated with Claude Code